Skip to content

[NFC][IR2Vec] Increasing tolerance in approximatelyEquals() of Embedding #145117

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

svkeerthy
Copy link
Contributor

@svkeerthy svkeerthy commented Jun 20, 2025

Increase the default tolerance for approximatelyEquals in IR2Vec's Embedding class from 1e-6 to 1e-4.

(Tracking issue - #141817)

@svkeerthy svkeerthy changed the title Increasing tolerance in ApproximatelyEquals [NFC][IR2Vec] Increasing tolerance in approximatelyEquals() of Embedding Jun 20, 2025
@svkeerthy svkeerthy requested a review from mtrofin June 20, 2025 23:32
@svkeerthy svkeerthy marked this pull request as ready for review June 20, 2025 23:33
@llvmbot llvmbot added mlgo llvm:analysis Includes value tracking, cost tables and constant folding labels Jun 20, 2025
@llvmbot
Copy link
Member

llvmbot commented Jun 20, 2025

@llvm/pr-subscribers-llvm-analysis

@llvm/pr-subscribers-mlgo

Author: S. VenkataKeerthy (svkeerthy)

Changes

Increase the default tolerance for approximatelyEquals in IR2Vec's Embedding class from 1e-6 to 1e-4.


Full diff: https://github.com/llvm/llvm-project/pull/145117.diff

2 Files Affected:

  • (modified) llvm/include/llvm/Analysis/IR2Vec.h (+1-1)
  • (modified) llvm/unittests/Analysis/IR2VecTest.cpp (+2-2)
diff --git a/llvm/include/llvm/Analysis/IR2Vec.h b/llvm/include/llvm/Analysis/IR2Vec.h
index 06312562060aa..480b834077b86 100644
--- a/llvm/include/llvm/Analysis/IR2Vec.h
+++ b/llvm/include/llvm/Analysis/IR2Vec.h
@@ -116,7 +116,7 @@ struct Embedding {
 
   /// Returns true if the embedding is approximately equal to the RHS embedding
   /// within the specified tolerance.
-  bool approximatelyEquals(const Embedding &RHS, double Tolerance = 1e-6) const;
+  bool approximatelyEquals(const Embedding &RHS, double Tolerance = 1e-4) const;
 
   void print(raw_ostream &OS) const;
 };
diff --git a/llvm/unittests/Analysis/IR2VecTest.cpp b/llvm/unittests/Analysis/IR2VecTest.cpp
index 05af55b59323b..33ac16828eb6c 100644
--- a/llvm/unittests/Analysis/IR2VecTest.cpp
+++ b/llvm/unittests/Analysis/IR2VecTest.cpp
@@ -154,14 +154,14 @@ TEST(EmbeddingTest, ApproximatelyEqual) {
   EXPECT_TRUE(E1.approximatelyEquals(E2)); // Diff = 1e-7
 
   Embedding E3 = {1.00002, 2.00002, 3.00002}; // Diff = 2e-5
-  EXPECT_FALSE(E1.approximatelyEquals(E3));
+  EXPECT_FALSE(E1.approximatelyEquals(E3, 1e-6));
   EXPECT_TRUE(E1.approximatelyEquals(E3, 3e-5));
 
   Embedding E_clearly_within = {1.0000005, 2.0000005, 3.0000005}; // Diff = 5e-7
   EXPECT_TRUE(E1.approximatelyEquals(E_clearly_within));
 
   Embedding E_clearly_outside = {1.00001, 2.00001, 3.00001}; // Diff = 1e-5
-  EXPECT_FALSE(E1.approximatelyEquals(E_clearly_outside));
+  EXPECT_FALSE(E1.approximatelyEquals(E_clearly_outside, 1e-6));
 
   Embedding E4 = {1.0, 2.0, 3.5}; // Large diff
   EXPECT_FALSE(E1.approximatelyEquals(E4, 0.01));

Copy link
Member

@mtrofin mtrofin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why - as in, expected because upcoming change; or flakyness?

@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-12-simplifying_creation_of_embedder branch from 8b8932b to 29ebe35 Compare June 23, 2025 21:10
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-20-increasing_tolerance_in_approximatelyequals branch 2 times, most recently from bf89c59 to 0472d10 Compare June 30, 2025 20:56
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-12-simplifying_creation_of_embedder branch 2 times, most recently from 037cc63 to cd4066f Compare June 30, 2025 21:10
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-20-increasing_tolerance_in_approximatelyequals branch from 0472d10 to b2c203a Compare June 30, 2025 21:11
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-12-simplifying_creation_of_embedder branch from cd4066f to f09b163 Compare July 1, 2025 01:11
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-20-increasing_tolerance_in_approximatelyequals branch 2 times, most recently from 6cf6937 to ec1d9d6 Compare July 1, 2025 01:20
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-12-simplifying_creation_of_embedder branch from f09b163 to f33b9e3 Compare July 1, 2025 01:20
Base automatically changed from users/svkeerthy/06-12-simplifying_creation_of_embedder to main July 1, 2025 01:24
@svkeerthy svkeerthy force-pushed the users/svkeerthy/06-20-increasing_tolerance_in_approximatelyequals branch from ec1d9d6 to 7516580 Compare July 1, 2025 01:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
llvm:analysis Includes value tracking, cost tables and constant folding mlgo
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants